Skip to content

Conversation

Dr-Irv
Copy link
Contributor

@Dr-Irv Dr-Irv commented Aug 30, 2025

Spent the time to make the code compliant with the most recent releases of mypy and pyright

@Dr-Irv Dr-Irv requested a review from rhshadrach as a code owner August 30, 2025 22:41
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!



def diff(arr, n: int, axis: AxisInt = 0):
def diff(arr, n: int | float | np.integer, axis: AxisInt = 0):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about np.floating as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, added in next commit.


# error: Incompatible return value type (got "bool_", expected "bool")
return self._mask.any() # type: ignore[return-value]
return cast(bool, self._mask.any())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this return a NumPy bool?

Copy link
Contributor Author

@Dr-Irv Dr-Irv Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, so I changed it to bool(self._mask.any())



def _validate_dialect(dialect: csv.Dialect) -> None:
def _validate_dialect(dialect: csv.Dialect | Any) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Any here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a pain to get right. Did something different in next commit.

There is a typeshed issue that I had to deal with. Reported that at python/typeshed#14667


@property
def shape(self) -> Shape | None:
def shape(self) -> Shape | list[int] | None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like this never returns Shape = tuple[int, ...]. Just do list[int] | None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in next commit

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Aug 31, 2025

@rhshadrach ready for another review.

# source code using it..

return cast(bool, self._mask.any())
return bool(self._mask.any())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, just noting that the implementations for other _hasna already have this bool(...) where appropriate.

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rhshadrach rhshadrach added Typing type annotations, mypy/pyright type checking CI Continuous Integration labels Sep 1, 2025
@rhshadrach rhshadrach added this to the 3.0 milestone Sep 1, 2025
@rhshadrach rhshadrach changed the title Upgrade mypy to 1.17.1 and pyright to 1.1.404 CI: Upgrade mypy to 1.17.1 and pyright to 1.1.404 Sep 1, 2025
@rhshadrach rhshadrach merged commit ef4a9c3 into pandas-dev:main Sep 1, 2025
46 checks passed
@rhshadrach
Copy link
Member

Thanks @Dr-Irv

eicchen pushed a commit to eicchen/pandas that referenced this pull request Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration Typing type annotations, mypy/pyright type checking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants